RDKEMW-17873: HDMI-CEC Polaris HAL Migration - #57
Conversation
apatel859
left a comment
There was a problem hiding this comment.
Can you cimpile this code in normal build and vdevice build and see if you face any compilation issue?
876022c to
1c009ec
Compare
|
All contributors have signed the CLA ✍️ ✅ |
02757ea to
ba51587
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a HAL abstraction + factory to switch the CCEC driver between a legacy C HAL backend (vHAL) and an Android Binder/AIDL backend (AidlHAL), and updates the build to produce/link the new backend libraries.
Changes:
- Added
HDMICecHalinterface andHDMICecHalFactoryto select AIDL vs legacy backend at runtime. - Implemented
vHAL(legacy C driver wrapper) andAidlHAL(binder/AIDL implementation). - Updated
DriverImplto route all HAL operations through the new interface; updated Makefiles to build/link backend shared libraries.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| ccec/src/Makefile.am | Builds libvHAL.la + libAidlHAL.la and links them into libRCEC.la. |
| ccec/src/Makefile | Builds libvHAL.so + libAidlHAL.so and links them into libRCEC.so; adds binder/AIDL include/link plumbing. |
| ccec/src/factoryImpl/HDMICecHal.h | New abstract HAL interface used by DriverImpl. |
| ccec/src/factoryImpl/HDMICecHalFactory.h | Declares factory for backend selection. |
| ccec/src/factoryImpl/HDMICecHalFactory.cpp | Implements backend selection via binder service discovery. |
| ccec/src/factoryImpl/vHAL.h | Declares legacy backend wrapper. |
| ccec/src/factoryImpl/vHAL.cpp | Implements legacy backend wrapper via HdmiCec* C APIs. |
| ccec/src/factoryImpl/AidlHAL.h | Declares AIDL/binder backend. |
| ccec/src/factoryImpl/AidlHAL.cpp | Implements AIDL/binder backend + poll ACK emulation + frame length filtering. |
| ccec/src/DriverImpl.hpp | Stores std::unique_ptr<HDMICecHal> for the selected backend. |
| ccec/src/DriverImpl.cpp | Uses HDMICecHal for open/close/read/write operations and AIDL-specific behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
174c2b5 to
1c06faf
Compare
32ad93b to
50d97a2
Compare
| class HalFactoryUtility { | ||
| enum class BackendType { | ||
| UNKNOWN, | ||
| LEGACY, | ||
| AIDL | ||
| }; | ||
|
|
||
| static BackendType mBackendType; | ||
|
|
||
| bool isAidlServiceAvailable(const android::String16 &expectedServiceName) | ||
| { | ||
| CCEC_LOG(LOG_INFO, "isAidlServiceAvailable invoked\r\n"); |
| { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory::Create invoked\r\n"); | ||
|
|
||
| if (HalFactoryUtility::isAidlServiceAvailable(IHdmiCec::serviceName().c_str())) { |
| const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8; | ||
| void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0); | ||
| if (mapped_mem == MAP_FAILED) { | ||
| CCEC_LOG(LOG_ERROR, "[-] Shared address space context instantiation failed\n"); |
| CCEC_LOG(LOG_DEBUG, "HDMICecRdkVHAL::getPhysicalAddress handle=%d ret=%d addr=0x%x\r\n", | ||
| ret, (physicalAddress ? *physicalAddress : 0)); |
| int HDMICecAidlHAL::getPhysicalAddress(int handle, unsigned int *physicalAddress) | ||
| { | ||
| if (physicalAddress != nullptr) { | ||
| *physicalAddress = 0; | ||
| } | ||
|
|
||
| CCEC_LOG( LOG_DEBUG, "HDMICecAidlHAL::getPhysicalAddress completed\r\n"); | ||
|
|
||
| return 0; | ||
| } |
| factoryImpl/HDMICecHalFactory.cpp \ | ||
| factoryImpl/HDMICecRdkVHAL.cpp \ | ||
| factoryImpl/HDMICecAidlHAL.cpp \ | ||
| factoryImpl/ServiceManagerCheck.cpp | ||
|
|
||
| libRCEC_la_LDFLAGS = -lpthread |
| factoryImpl/HDMICecHalFactory.cpp \ | ||
| factoryImpl/HDMICecRdkVHAL.cpp \ | ||
| factoryImpl/HDMICecAidlHAL.cpp \ | ||
| factoryImpl/ServiceManagerCheck.cpp |
| CCEC_LOG(LOG_INFO, "[+] Binder protocol version detected: %d\n", version.protocol_version); | ||
|
|
||
| const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8; | ||
| void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0); |
| // Macro definitions for internal use | ||
| #define BINDER_VERSION _IOWR('b', 9, struct binder_version) | ||
| #define BINDER_WRITE_READ_V7 _IOWR('b', 1, struct binder_write_read_v7) | ||
| #define BC_TRANSACTION_V7 _IOW('c', 0, struct binder_transaction_data_v7) |
| bwr.write_size = write_size; | ||
| bwr.write_consumed = 0; | ||
| bwr.write_buffer = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(tx.write_payload.data())); | ||
| bwr.read_size = read_size; | ||
| bwr.read_consumed = 0; | ||
| bwr.read_buffer = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(tx.read_payload.data())); |
| /* Probe with a 2-byte directed frame (GiveDevicePowerStatus) */ | ||
| { | ||
| AutoLock lock_(mAidlMutex); | ||
| std::vector<uint8_t> probe; | ||
| probe.reserve(2); | ||
| probe.push_back(buf ? buf[0] : 0); | ||
| probe.push_back(0x8F); // GiveDevicePowerStatus | ||
|
|
||
| SendMessageStatus probeStatus = SendMessageStatus::BUSY; | ||
| android::binder::Status aidlStatus = mAidlController->sendMessage(probe, &probeStatus); | ||
| if (aidlStatus.isOk() && probeStatus == SendMessageStatus::ACK_STATE_0) { |
| /** | ||
| * @brief Get the physical address of the device. | ||
| * | ||
| * Legacy: calls HdmiCecGetPhysicalAddress(). | ||
| * AIDL: queries the physical address via binder. | ||
| * |
| std::unique_ptr<IHDMICecHal> HDMICecHalFactory::Create() | ||
| { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory::Create invoked\r\n"); | ||
|
|
||
| try { | ||
| if (HalFactoryUtility::isAidlServiceAvailable(android::String16(IHdmiCec::serviceName().c_str()))) { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory: Aidl Service is available — using HDMICecAidlHAL\r\n"); | ||
| return std::make_unique<HDMICecAidlHAL>(); | ||
| } |
|
I have read the CLA Document and I hereby sign the CLA. |
|
|
||
| ln -sf ../../../mocks/hdmicec/hdmi_cec_driver.h stubs/ccec/drivers/hdmi_cec_driver.h | ||
|
|
||
| cat <<'EOF' > stubs/utils/String16.h |
There was a problem hiding this comment.
Headerfiles should not created from workflows. It should be maintained in stubs folder as original source file. So Can we remove header file creation here. Move it to stubs.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
| #define BINDER_VERSION _IOWR('b', 9, struct binder_version) | ||
| #define BINDER_WRITE_READ_V7 _IOWR('b', 1, struct binder_write_read_v7) | ||
| #define BC_TRANSACTION_V7 _IOW('c', 0, struct binder_transaction_data_v7) | ||
|
|
| const size_t tx_words = sizeof(txn) / sizeof(uint32_t); | ||
| tx.write_payload.reserve(1 + tx_words); | ||
| tx.write_payload.push_back(BC_TRANSACTION_V7); | ||
|
|
||
| tx.write_payload.resize(1 + tx_words); |
| const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8; | ||
| void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0); | ||
| if (mapped_mem == MAP_FAILED) { | ||
| CCEC_LOG(LOG_ERROR, "[-] Shared address space context instantiation failed\n"); |
| int HDMICecAidlHAL::getPhysicalAddress(int handle, unsigned int *physicalAddress) | ||
| { | ||
| if (physicalAddress != nullptr) { | ||
| *physicalAddress = 0; | ||
| } |
| void HDMICecAidlHAL::dispatchRx(unsigned char *buf, int len) | ||
| { | ||
| if (mRxCb == nullptr) { | ||
| CCEC_LOG(LOG_DEBUG, "HDMICecAidlHAL::dispatchRx callback not registered\r\n"); | ||
| return; | ||
| } | ||
|
|
||
| // Track initiator LA from inbound frames so 1-byte poll can be emulated | ||
| // locally on AIDL backends that reject 1-byte sendMessage payloads. | ||
| if (buf != nullptr && len >= 1) { | ||
| const uint8_t srcLA = static_cast<uint8_t>((buf[0] >> 4) & 0x0F); | ||
| if (srcLA <= 0x0E) { | ||
| AutoLock lock_(mAidlMutex); | ||
| mSeenLogicalAddresses.insert(srcLA); | ||
| } | ||
| } | ||
| mRxCb(0, mRxCbData, buf, len); | ||
| } |
| int HDMICecAidlHAL::setRxCallback(int handle, HdmiCecRxCallback_t cbfunc, void *data) | ||
| { | ||
| (void)handle; | ||
|
|
||
| mRxCb = cbfunc; | ||
| mRxCbData = data; | ||
|
|
||
| CCEC_LOG(LOG_DEBUG, "HDMICecAidlHAL::setRxCallback invoked\r\n"); | ||
| return 0; | ||
| } |
| int HDMICecAidlHAL::setTxCallback(int handle, HdmiCecTxCallback_t cbfunc, void *data) | ||
| { | ||
| (void)handle; | ||
|
|
||
| mTxCb = cbfunc; | ||
| mTxCbData = data; | ||
|
|
||
| CCEC_LOG(LOG_DEBUG, "HDMICecAidlHAL::setTxCallback invoked\r\n"); | ||
| return 0; | ||
| } |
| /* Probe with a 2-byte directed frame (GiveDevicePowerStatus) */ | ||
| { | ||
| AutoLock lock_(mAidlMutex); | ||
| std::vector<uint8_t> probe; | ||
| probe.reserve(2); | ||
| probe.push_back(buf ? buf[0] : 0); | ||
| probe.push_back(0x8F); // GiveDevicePowerStatus | ||
|
|
||
| SendMessageStatus probeStatus = SendMessageStatus::BUSY; | ||
| android::binder::Status aidlStatus = mAidlController->sendMessage(probe, &probeStatus); | ||
| if (aidlStatus.isOk() && probeStatus == SendMessageStatus::ACK_STATE_0) { | ||
| mSeenLogicalAddresses.insert(destination); | ||
| CCEC_LOG(LOG_DEBUG, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames destination=0x%X ACKed by 2-byte probe. Emulating ack.\r\n", | ||
| destination); | ||
| return true; | ||
| } | ||
|
|
||
| CCEC_LOG(LOG_DEBUG, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames destination=0x%X probe NACK/failed (aidlOk=%d status=%d).\r\n", | ||
| destination, | ||
| aidlStatus.isOk() ? 1 : 0, | ||
| static_cast<int>(probeStatus)); | ||
| } |
| - name: Generate stub headers | ||
| # Empty headers to mute errors | ||
| run: > | ||
| cd "$GITHUB_WORKSPACE/hdmicec" | ||
| && | ||
| mkdir -p | ||
| stubs/rdk/iarmbus | ||
| stubs/ccec/drivers/iarmbus | ||
| stubs/binder | ||
| stubs/utils | ||
| stubs/linux/android | ||
| stubs/com/rdk/hal/hdmicec | ||
| && |
| std::unique_ptr<IHDMICecHal> HDMICecHalFactory::Create() | ||
| { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory::Create invoked\r\n"); | ||
|
|
||
| try { | ||
| if (HalFactoryUtility::isAidlServiceAvailable(android::String16(IHdmiCec::serviceName().c_str()))) { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory: Aidl Service is available — using HDMICecAidlHAL\r\n"); | ||
| return std::make_unique<HDMICecAidlHAL>(); | ||
| } | ||
| } catch (...) { | ||
| CCEC_LOG(LOG_ERROR, "HDMICecHalFactory: Exception thrown while creating AIDL HAL,\r\n"); | ||
| } | ||
|
|
||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory: Aidl Service is not available — using legacy HDMICecRdkVHAL\r\n"); | ||
| return std::make_unique<HDMICecRdkVHAL>(); | ||
| } |
248be82 to
5e1ed8e
Compare
5e1ed8e to
3dfa44b
Compare
|
|
||
| const size_t tx_words = sizeof(txn) / sizeof(uint32_t); | ||
| tx.write_payload.reserve(1 + tx_words); | ||
| tx.write_payload.push_back(BC_TRANSACTION_V7); |
| int HDMICecAidlHAL::getPhysicalAddress(int handle, unsigned int *physicalAddress) | ||
| { | ||
| if (physicalAddress != nullptr) { | ||
| *physicalAddress = 0; | ||
| } | ||
|
|
||
| CCEC_LOG( LOG_DEBUG, "HDMICecAidlHAL::getPhysicalAddress completed\r\n"); | ||
|
|
||
| return 0; | ||
| } |
| std::unique_ptr<IHDMICecHal> HDMICecHalFactory::Create() | ||
| { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory::Create invoked\r\n"); | ||
|
|
||
| try { | ||
| if (HalFactoryUtility::isAidlServiceAvailable(android::String16(IHdmiCec::serviceName().c_str()))) { | ||
| CCEC_LOG(LOG_INFO, "HDMICecHalFactory: Aidl Service is available — using HDMICecAidlHAL\r\n"); | ||
| return std::make_unique<HDMICecAidlHAL>(); | ||
| } |
3dfa44b to
4a9145c
Compare
76022a7 to
3097ab2
Compare
| static bool execute_binder_ping(const int binder_fd, const int protocol_version) { | ||
| const BinderTransaction tx = (protocol_version == 7) ? prepare_v7_transaction() : prepare_v8_transaction(); | ||
| uint32_t bytes_consumed = 0; |
| const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8; | ||
| void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0); | ||
| if (mapped_mem == MAP_FAILED) { | ||
| CCEC_LOG(LOG_ERROR, "[-] Shared address space context instantiation failed\n"); | ||
| close(binder_fd); | ||
| return service_manager_alive; | ||
| } |
| binder_write_read_v7 bwr{}; | ||
| const size_t write_size = tx.write_payload.size() * sizeof(uint32_t); | ||
| const size_t read_size = tx.read_payload.size() * sizeof(uint32_t); | ||
| bwr.write_size = write_size; | ||
| bwr.write_consumed = 0; | ||
| bwr.write_buffer = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(tx.write_payload.data())); | ||
| bwr.read_size = read_size; | ||
| bwr.read_consumed = 0; | ||
| bwr.read_buffer = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(tx.read_payload.data())); | ||
|
|
| int HDMICecAidlHAL::getPhysicalAddress(int handle, unsigned int *physicalAddress) | ||
| { | ||
| if (physicalAddress != nullptr) { | ||
| *physicalAddress = 0; | ||
| } | ||
|
|
||
| CCEC_LOG( LOG_DEBUG, "HDMICecAidlHAL::getPhysicalAddress completed\r\n"); | ||
|
|
||
| return 0; | ||
| } |
| bool HDMICecAidlHAL::emulateAckForPollFrames(const unsigned char *buf, int len) | ||
| { | ||
| if (len <= 1) { | ||
| /* | ||
| * Poll frame (header only): emulate ACK based on seen-LA cache | ||
| * and 2-byte probe. This keeps HdmiCecSource ping-based discovery | ||
| * working on AIDL backend. | ||
| */ | ||
| const uint8_t destination = (buf != NULL) ? (buf[0] & 0x0F) : 0xFF; | ||
|
|
||
| if (destination <= 0x0E) { | ||
| /* Check seen-LA cache first */ | ||
| { | ||
| AutoLock lock_(mAidlMutex); | ||
| if (mSeenLogicalAddresses.count(destination) > 0) { | ||
| CCEC_LOG(LOG_DEBUG, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames destination=0x%X present in seen-LA set. Emulating ack.\r\n", | ||
| destination); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| /* Probe with a 2-byte directed frame (GiveDevicePowerStatus) */ | ||
| { | ||
| AutoLock lock_(mAidlMutex); | ||
| std::vector<uint8_t> probe; | ||
| probe.reserve(2); | ||
| probe.push_back(buf ? buf[0] : 0); | ||
| probe.push_back(0x8F); // GiveDevicePowerStatus | ||
|
|
||
| SendMessageStatus probeStatus = SendMessageStatus::BUSY; | ||
| android::binder::Status aidlStatus = mAidlController->sendMessage(probe, &probeStatus); | ||
| if (aidlStatus.isOk() && probeStatus == SendMessageStatus::ACK_STATE_0) { | ||
| mSeenLogicalAddresses.insert(destination); | ||
| CCEC_LOG(LOG_DEBUG, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames destination=0x%X ACKed by 2-byte probe. Emulating ack.\r\n", | ||
| destination); | ||
| return true; | ||
| } | ||
|
|
||
| CCEC_LOG(LOG_DEBUG, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames destination=0x%X probe NACK/failed (aidlOk=%d status=%d).\r\n", | ||
| destination, | ||
| aidlStatus.isOk() ? 1 : 0, | ||
| static_cast<int>(probeStatus)); | ||
| } | ||
| } | ||
|
|
||
| CCEC_LOG(LOG_DEBUG, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames destination=0x%X not present. Returning no-ack.\r\n", | ||
| (buf != NULL) ? (buf[0] & 0x0F) : 0xFF); | ||
| throw CECNoAckException(); | ||
| } | ||
|
|
||
| if (static_cast<size_t>(len) > kAidlMaxCecFrameSize) { | ||
| CCEC_LOG(LOG_EXP, | ||
| "HDMICecAidlHAL::emulateAckForPollFrames blocking unsupported CEC frame length=%zu on AIDL backend (valid range: 2..16).\r\n", | ||
| static_cast<size_t>(len)); | ||
| throw IOException(); | ||
| } | ||
|
|
||
| return false; | ||
| } |
| #include <cstdint> | ||
| #include <cstdlib> |
| CCEC_LOG(LOG_INFO, "isAidlServiceAvailable invoked\r\n"); | ||
|
|
||
| if (mBackendType == BackendType::AIDL) { | ||
| return true; | ||
| } else if (mBackendType == BackendType::LEGACY) { | ||
| return false; | ||
| } |
| #define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read) | ||
| #define BC_TRANSACTION 0x0 | ||
| #define TF_ACCEPT_FDS 0x10 | ||
| #define BR_REPLY 0x1 | ||
| #define BR_DEAD_REPLY 0x2 | ||
| #define BR_FAILED_REPLY 0x3 | ||
| #define BR_TRANSACTION_COMPLETE 0x4 | ||
| #define BR_NOOP 0x5 | ||
| #define BR_OK 0x6 |
| # Calculate AIDL include path relative to workspace | ||
| AIDL_GEN_DIR := $(shell cd ../../.. && pwd)/aidl/rdk-halif-aidl/gen/hdmicec/current | ||
| AIDL_H_DIR := $(AIDL_GEN_DIR)/h | ||
| BINDER_IDL_DIR := $(shell cd ../../.. && pwd)/aidl/rdk-halif-aidl/build-tools/linux_binder_idl | ||
| BINDER_INCLUDE := $(BINDER_IDL_DIR)/android/native/libs/binder/include |
| // Return the standard logical-address candidates for a given device type. | ||
| // Used only as a fallback when AIDL reports no allocated addresses yet. | ||
| std::vector<int32_t> preferredLogicalAddressesForDeviceType(int devType) | ||
| { |
| int HDMICecAidlHAL::getPhysicalAddress(int handle, unsigned int *physicalAddress) | ||
| { | ||
| if (physicalAddress != nullptr) { | ||
| *physicalAddress = 0; | ||
| } | ||
|
|
||
| CCEC_LOG( LOG_DEBUG, "HDMICecAidlHAL::getPhysicalAddress completed\r\n"); | ||
|
|
||
| return 0; | ||
| } |
| private: | ||
| const size_t kAidlMinCecFrameSize = 2; | ||
| const size_t kAidlMaxCecFrameSize = 16; | ||
| android::sp<com::rdk::hal::hdmicec::IHdmiCec> getAidlService(); |
No description provided.